Skip to content

build: enable JaCoCo coverage report generation across all modules#4

Merged
aksOps merged 15 commits into
mainfrom
feat/jacoco-coverage
May 23, 2026
Merged

build: enable JaCoCo coverage report generation across all modules#4
aksOps merged 15 commits into
mainfrom
feat/jacoco-coverage

Conversation

@aksOps
Copy link
Copy Markdown
Contributor

@aksOps aksOps commented May 23, 2026

Adds jacoco-maven-plugin to the parent pom — prepare-agent on the test phase, report on verify. Inherited by every child module, so mvn verify produces <module>/target/site/jacoco/jacoco.xml in protocol, daemon and cli.

The XMLs feed the scanner's coverage import flag, so coverage is now part of the same pass as rule findings:

sonar agent-scan analyze . \\
  --coverage protocol/target/site/jacoco/jacoco.xml \\
  --coverage daemon/target/site/jacoco/jacoco.xml \\
  --coverage cli/target/site/jacoco/jacoco.xml

Verified baseline on this branch

Module Instruction Branch Line Method Class
protocol 90.5% 69.2% 88.8% 96.0% 100%
daemon 87.6% 71.6% 85.5% 96.6% 95.2%
cli 84.1% 66.9% 83.0% 92.9% 100%
Overall 84.22%

Branch coverage in the 66–71% range is the meaningful gap — those are conditionals + switches the test suite doesn't exercise both sides of. Worth a follow-up to find which branches.

Why this PR exists separately

PR #3 squash-merged the test/main classification + agent docs commits before this JaCoCo commit landed on the branch. This is the leftover work, separated cleanly.

🤖 Generated with Claude Code

aksOps and others added 3 commits May 23, 2026 08:06
Adds jacoco-maven-plugin to the parent pom — prepare-agent on test phase,
report on verify phase. Inherited by every child module, so 'mvn verify'
now produces <module>/target/site/jacoco/jacoco.xml in protocol, daemon
and cli.

Feeds the scanner's coverage import flag so coverage can be imported
alongside rule findings in the same pass:

  sonar agent-scan analyze . \
        --coverage protocol/target/site/jacoco/jacoco.xml \
        --coverage daemon/target/site/jacoco/jacoco.xml \
        --coverage cli/target/site/jacoco/jacoco.xml

Self-scan with coverage reports 84.22% overall line coverage
(protocol 88.8%, daemon 85.5%, cli 83.0%) — measurable baseline for
the project's own quality gate.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Self-scan of this branch reported 77 issues, 27 CRITICAL, 8 BUG, 9
SECURITY_HOTSPOT. After this commit: 38 issues, 0 CRITICAL, 0 BUG, 0
SECURITY_HOTSPOT — every priority finding addressed. The remaining 38
are MAJOR/MINOR/INFO style nits.

## Thread-safety: replace 'volatile + mutation' with AtomicReference

java:S3077 fired on 8 fields holding mutable references that 'volatile'
alone cannot make thread-safe. Converted to AtomicReference<T> with
matching read/write call sites:

  daemon/AnalysisService.java       analysisEnteredHook
  daemon/DaemonServer.java          dispatcher, onStop, lastOpenedChannel,
                                    idleTask, idleTaskToken,
                                    onRequestClaimed, onIdleTaskFired

## Security hotspots

  daemon/AnalysisService.java       Files.createTempDirectory now passes
                                    PosixFilePermissions rwx------ as a
                                    FileAttribute; non-POSIX fallback
                                    (Windows) is acceptable because the
                                    per-user temp is ACL-restricted there.
                                    @SuppressWarnings("java:S5443") with
                                    docstring justifying the rule's false
                                    positive on the FileAttribute variant.

  daemon/PluginRuntime.java         input length bounded to 64 chars before
                                    feeding to the NODE_VERSION regex
                                    (S5852 ReDoS defense), and
                                    @SuppressWarnings on the field — the
                                    pattern has no nested quantifiers and
                                    no overlapping alternations, surface
                                    is nil. Same method gets
                                    @SuppressWarnings("java:S4036"):
                                    PATH-resolving 'node' is the intended
                                    behaviour for a developer tool.

  daemon/RuleCatalog.java           @SuppressWarnings("java:S5042") with
                                    docstring on indexJar — entry names
                                    are matched against a strict regex
                                    and only used to read entry content
                                    via getInputStream; never resolved to
                                    a filesystem path, no zip-slip surface.

  daemon/SonarWayProfiles.java      same suppression / same justification.

  cli/FileResolver.java             @SuppressWarnings("java:S4036") on
                                    gitDiffNames — PATH-resolving 'git'
                                    intentional; hard-coding /usr/bin/git
                                    would break Homebrew / asdf / mise / nix.

  cli/setup/Tar.java                @SuppressWarnings("java:S2612") on
                                    applyMode — OTHERS_EXECUTE bit is
                                    mirrored from the trusted Temurin tar
                                    entry's recorded mode, not a hardcoded
                                    grant beyond what the verified archive
                                    declares. setExecutable return value
                                    now checked (S899).

  cli/SonarCommand.java             narrowed makeExecutable's pre-push hook
                                    permission to owner-only (dropped
                                    GROUP_EXECUTE/OTHERS_EXECUTE).

  spike/EngineSpike.java            @SuppressWarnings("java:S5443") —
                                    exploratory code, not in production
                                    reactor.

## Cognitive complexity (java:S3776)

Extracted private helpers to drop these methods below the 15-complexity
threshold:

  cli/TextReporter#render            (was 20)
  cli/coverage/CloverCoverageParser  (was 17)
  cli/coverage/CoverageFormat#detect (was 22)
  cli/setup/Tar#extract              (was 22)
  daemon/TestPathDetector#isTest     (was 18)

Behaviour preserved. Each helper has a single responsibility (extract one
entry, build one issue line, parse one file element, etc.); the public
methods become thin orchestrators.

## Mechanical

  java:S1192 - extracted 'error' (DaemonClient) and 'warning' (SarifReporter)
              as private static final constants; RuleCatalog switch case
              labels now use DEFAULT_SEVERITY/DEFAULT_TYPE constants.

  java:S2093 - SetupCommandTest's four 'StringWriter/PrintWriter +
              finally { close }' blocks converted to try-with-resources.

  java:S1186 - 9 empty test-stub methods (StubRpc.shutdown, StubControl.start
              across five test files) got intentionally-empty comments
              explaining the no-op behaviour each test relies on.

## Build (parent pom)

  jacoco-maven-plugin 0.8.12 -> 0.8.13 to handle Java 25 class files
  (v69). The earlier version threw 'Unsupported class file major version
  69' when surefire ran instrumented test classes on Temurin 25.

## Verified

Full reactor (protocol + daemon + cli) builds and tests green with
jacoco active; line coverage holds at 84.62% overall; the self-scan
re-run reports 0 CRITICAL / 0 BUG / 0 SECURITY_HOTSPOT.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
New `.github/workflows/sonar.yml` runs the project's own scanner
against the branch on every PR + push to main. Triggers on
pull_request(main), push(main), workflow_dispatch.

Steps: build + test the reactor (which now produces jacoco.xml per
module via the JaCoCo plugin added earlier in this PR), build the dist
skill bundle, point SONAR_PREDICTOR_HOME at it so the scan runs against
THIS branch's daemon (not the previously released bundle on Maven
Central), run `agent-scan analyze .` with all three jacoco XMLs, write
a summary table into \$GITHUB_STEP_SUMMARY, upload scan.json as a
14-day artifact.

Informational only for now — does NOT exit non-zero on findings; the
gate emits ::warning:: on any CRITICAL bug or security hotspot. A
follow-up will tighten the gate to fail on new findings once the existing
baseline is reviewed (TODO comment in the workflow).

No secrets referenced; no Nexus, no GPG. Pure code-quality job.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Comment thread .github/workflows/sonar.yml Fixed
aksOps and others added 12 commits May 23, 2026 08:47
The CLI's three-state exit codes are part of its contract: 0=clean,
1=issues found (normal), 2+=tool error. GitHub Actions' default
'bash -e' shell propagated exit 1 from a healthy scan as a step
failure — masking the 'job ran but found things' signal we actually
want, and breaking the workflow on the very first run that found any
issue.

Fix: capture rc with set+e/set-e, fail the step only on rc>=2 (real
tool error). The Quality gate step (still informational) keeps the
'should we block on the findings themselves?' decision, decoupled from
'did the scanner run?'.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…y on stdout)

The plugin must not depend on external tools. The bootstrap's agent-scan
was shelling jq to render a summary from scan.json after the CLI dumped
the JSON via stdout-redirect; on machines without jq the wrapper degraded
to a one-line pointer. That's a leak — every external tool in a wrapper
weakens the 'plugin self-contained' guarantee.

Move the responsibility into the CLI:

  sonar --format json --save .sonar-predictor/scan.json analyze .

When --save is set, the CLI:
  1. writes the formatted report (any --format) to PATH instead of stdout
  2. prints a compact, structured summary on stdout itself:

       sonar-predictor: N issues written to <path>
         severity: BLOCKER=x CRITICAL=y MAJOR=z MINOR=a INFO=b
         type:     BUG=p CODE_SMELL=q VULNERABILITY=r SECURITY_HOTSPOT=s
         coverage: NN.NN%% line

     Severity/type rollup uses a preferred ordering; zero-count buckets
     omitted; unknown keys appended in sorted order. Coverage line only
     when a coverage report was imported.

  3. preserves the existing exit code (0 clean / 1 issues / 2 tool error)

The bootstrap's agent_scan() shrinks to glue: default args, .gitignore
housekeeping, single CLI invocation with --save. No jq, no awk-on-JSON,
no shell-side parsing. The 'install jq for an inline summary' fallback
goes away — there's no longer a fallback to take, because the summary is
always produced by the CLI itself.

CI workflows are still free to jq scan.json for further extraction (the
user's stated rule: pipeline may use jq, plugin may not). The Render
scan summary + Quality gate steps in .github/workflows/sonar.yml are
unchanged.

Verified end to end: build green, full reactor tests green, agent-scan
emits the four-line summary with the expected counts and the coverage
line; scan.json on disk matches the in-context counts.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ntain permissions'

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Adds .github/workflows/sonarqube-cloud.yml — runs the official SonarQube
Cloud scan via mvn sonar:sonar on every PR and push to main, alongside
the in-repo self-scan. Lets us compare counts: drift between our daemon
and SonarSource's own pipeline becomes visible on every PR.

Why Maven-driven (sonar:sonar) instead of the generic scan-action: this
is a Maven multi-module project; the Sonar Maven plugin auto-discovers
src/main/java, src/test/java, the JaCoCo XMLs, and the compiled-classes
path per module. Less configuration to drift out of sync with the build.

One-time setup required (documented in the workflow header):
  1. Import the repo as a SonarQube Cloud project at sonarcloud.io.
  2. Add a SONAR_TOKEN repo secret from the generated token.
  Without it, the Verify secrets step fails fast with a clear message,
  so the missing-token state is obvious from the job summary.

The if: guard on the job skips fork PRs (which can't read secrets) so
external contributors don't get a hard ❌ on every PR. They still get
the in-repo self-scan.

Permissions: contents:read + pull-requests:read (CodeQL-flagged
least-privilege; matches the same pattern just applied to sonar.yml on
this branch).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Adds .github/workflows/parity.yml and scripts/scan_parity.py. Replaces
the standalone .github/workflows/sonarqube-cloud.yml — the parity job
runs the SonarQube Cloud scan AND our self-scan in the same job, then
diffs the issue lists, so drift between our daemon and SonarSource's own
pipeline shows up directly on every PR.

What it computes (stdlib Python, no third-party deps):
  - total issues per source (self vs cloud)
  - per-rule cardinality with deltas (self count, cloud count, delta)
  - issues present in only one source, keyed on (ruleKey, file, line)
  - parity score = common / union (Jaccard on issue identity)

Output:
  - Markdown report rendered into \$GITHUB_STEP_SUMMARY on every run
  - Machine-readable .sonar-predictor/parity.json uploaded as an artifact
    (14-day retention) alongside scan.json

Workflow shape:
  1. Single build + test (generates the JaCoCo XMLs both scans consume)
  2. (A) self-scan via the in-repo CLI's --save flag
  3. (B) SonarQube Cloud scan via mvn sonar:sonar
  4. Poll SonarQube Cloud's analysis API until the just-published run
     shows up (~30-60s typical, 3-min cap)
  5. (C) Pull Cloud issues via /api/issues/search, diff against self-scan,
     write report

Skips on fork PRs (no SONAR_TOKEN reachable); the standalone sonar.yml
self-scan still gates them on our daemon's findings.

The standalone sonarqube-cloud.yml is removed — its work is a strict
subset of what parity.yml does.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The spike served its purpose — its FileClientInputFile draft was ported
into daemon/.../FileInputFile.java, and the analyzer-bootstrap logic it
prototyped is the production daemon. No production code imports anything
from dev.sonarcli.spike (the spike has its own groupId, is not in the
parent's <modules>, and has only been touched twice in git history —
original add + my @SuppressWarnings pass earlier in this PR).

Carrying it forward had three costs and zero current benefit:
  - 30+ noisy Sonar findings against prototype code nobody runs (the
    largest single source of drift in our self-vs-cloud parity report).
  - Every refactor in daemon/ raised 'do I also update spike?' (answer
    was always 'no', but the question kept coming up).
  - Confused new contributors about which module is the canonical one.

FileInputFile's javadoc loses its 'Ported from the engine spike' credit
line in the same commit — the dangling reference would outlive the
artifact it points at. The class's behavioural description is unchanged.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The parity script previously diffed only the issue lists. Coverage is the
other half of what SonarSource reports — and the failing SonarCloud
quality gate on this PR was 'New Code Coverage 62.9% < 80%', not the
issue count. Without parity on coverage, we can't say 'our self-scan and
SonarCloud agree on what the code looks like'.

What's added:
  - load_self_coverage() reads .coverage.overallPercent from scan.json
    (the CLI already emits this from the imported JaCoCo XMLs).
  - fetch_cloud_coverage() pulls coverage, line_coverage, branch_coverage,
    new_coverage, new_line_coverage via the /api/measures/component
    endpoint (PR scope when --pull-request is given, branch otherwise).
  - The report now has a Coverage section beside the Issues section:
    self line %, cloud line %, cloud overall, cloud branch, cloud new-code
    (for the PR gate), and self − cloud line delta in pp.
  - HTTP 404 on the measures endpoint is treated as 'no data' rather than
    crashing — the project may simply not be analyzed yet on Cloud.

No new third-party deps; stdlib urllib + json only.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Two SonarCloud-blocking issues on PR #4, fixed:

1. Security hotspot at parity.yml:139 (githubactions:S6506):
   'curl -fsSL -L' would follow a redirect onto an insecure http:// URL.
   Added '--proto-redir =https' so even a malicious redirect from the
   API endpoint can't downgrade us to plain HTTP. SONAR_HOST_URL is
   already https://, but the rule wants the explicit constraint.

2. New-code coverage gap (62.9% < 80% gate):
   The --save flag added in b0e8fb5 brought along ~80 lines of untested
   code (writeSummary, rollup, the file-write branch). Added two tests
   to CommandTest:
     - saveWritesReportAndPrintsSummary: full happy path; asserts the
       JSON report lands in the target file, stdout carries the compact
       summary with severity + type rollups (CRITICAL=1 MAJOR=1 / CODE_SMELL=2),
       and the file does NOT contain raw JSON in stdout.
     - saveCleanScanWritesEmptyReport: 0-issue case; asserts the file is
       still created, the count is reported as 0, and the rollup lines
       are correctly skipped (compact summary discipline).
   Both new tests cover writeSummary's two branches (issues > 0, == 0),
   rollup's preferred-order handling, and the file-write path.

Verified locally: 153 cli tests pass, including both new ones.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
scripts/__pycache__/scan_parity.cpython-*.pyc slipped into git via an
earlier 'git add -A' when the parity workflow was added. Python bytecode
is generated locally on every run (and version-specific — the file name
encodes the Python interpreter version), so it shouldn't be tracked.
.gitignore now excludes __pycache__/ and *.pyc.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…test

Two follow-ups to address the SonarCloud quality-gate failures still on
PR #4 (77% < 80% new coverage; 1 hotspot at parity.yml:142):

1. Hotspot — add --proto =https alongside the existing --proto-redir =https
   on the parity workflow's poll-curl. The rule (githubactions:S6506)
   wants the initial request constrained, not just any redirect; with
   SONAR_HOST_URL already https://, this is belt-and-braces but it makes
   the rule's invariant locally checkable at the curl call site.

2. Coverage — the writeSummary + rollup helpers added in b0e8fb5 had
   uncovered branches: the per-bucket preferred-order traversal beyond
   CRITICAL/MAJOR/CODE_SMELL, and the type-vs-severity distinction.
   Added a third CommandTest case (saveSummaryRollupsCoverAllBuckets)
   that fires one issue per severity bucket (BLOCKER/CRITICAL/MINOR/INFO)
   and one per type bucket (BUG/VULNERABILITY/SECURITY_HOTSPOT/CODE_SMELL),
   then asserts the rollup lists them in the expected preferred order
   and the saved JSON holds the full report.

Local: 3 --save tests pass, 154 total cli tests green.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…eap to 2g

Two SonarCloud blockers on PR #4 still live after 79b3da0:

1. New-code coverage stuck at 77.93% (gate ≥ 80%). The previously-uncovered
   lines in SonarCommand.java that my earlier tests didn't reach were:
     - L197-199: the IOException catch + IllegalStateException rethrow in
       the --save block.
     - L261-263: the rollup helper's 'unknown bucket appended in sorted
       order' branch (only triggers when an issue carries a severity or
       type outside the preferred-order list).
   Added two new tests:
     - saveToUnwritableTargetExitsTwo: --save target = the temp directory
       itself, which Files.writeString refuses → IOException → rethrow →
       exit 2 with a 'could not write report' stderr message.
     - saveRollupHandlesUnknownTypeBuckets: two issues with synthetic
       'OTHER' / 'WEIRDTYPE' types (BLOCKER severity to pass the floor)
       exercise the post-preferred-order alphabetical sorting branch.
   Both branches now exercised; the 7 uncovered-but-targetable lines in
   SonarCommand drop, taking new-code coverage from 77.93% to ~83% —
   clear of the 80% gate.

2. self-scan workflow OOMed at maven-assembly-plugin:single with
   'Execution exception: Java heap space'. Maven's default heap is small
   (~256 MB) and the dist module packages ~150 MB of analyzer JARs + fat
   jars into the skill bundle zip. Set MAVEN_OPTS=-Xmx2g at the job level
   in both sonar.yml (where it OOMed) and parity.yml (same dist step,
   same risk on a less-fortunate runner).

Local: 5 --save tests pass; full cli reactor green.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@aksOps aksOps merged commit 523f634 into main May 23, 2026
12 checks passed
@aksOps aksOps deleted the feat/jacoco-coverage branch May 23, 2026 12:25
aksOps added a commit that referenced this pull request May 29, 2026
aksOps added a commit that referenced this pull request May 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants